Skip to content

Add support for nested eager loading on namespaced relation classes#1

Open
lucasmartins wants to merge 1 commit intomasterfrom
fix-nested-eager-loading
Open

Add support for nested eager loading on namespaced relation classes#1
lucasmartins wants to merge 1 commit intomasterfrom
fix-nested-eager-loading

Conversation

@lucasmartins
Copy link

I was getting the following error when doing nested eager loading, when relations are not the default belongs_to :other_thing:

NameError: uninitialized constant OtherClass
from /Users/lucasmartins/.gem/ruby/2.3.1/gems/activesupport-4.2.6/lib/active_support/inflector/methods.rb:261:in `const_get'

This is a relation like I'm using:

class NS::ModuleName::MyClass
  belongs_to :other_class, class_name: NS::ModuleName::OtherClass.to_s, index: true, touch: true
end

class NS::ModuleName::OtherClass
  has_many :my_class, class_name: NS::ModuleName::MyClass.to_s
end

While debugging the code, I've noticed that get_inclusion_metadata was trying to constantize "OtherClass" instead of "NS::ModuleName::OtherClass".

# lib/mongoid/criteria/includable.rb:134
def get_inclusion_metadata(_klass, association)
  if _klass.is_a?(Class)
    _klass.reflect_on_association(association)
  else
    # here, _klass.to_s.classify was "OtherClass", while it should be "NS::ModuleName::OtherClass"
    _klass.to_s.classify.constantize.reflect_on_association(association)
  end
end

I'm not sure about performance impact though.

I need somebody who knows this codebase better to check it out.

Any thoughts?

I was getting the following error when doing [nested eager loading](https://jira.mongodb.org/browse/MONGOID-4173), when relations are not the default `belongs_to :other_thing`:

```
NameError: uninitialized constant OtherClass
from /Users/lucasmartins/.gem/ruby/2.3.1/gems/activesupport-4.2.6/lib/active_support/inflector/methods.rb:261:in `const_get'
```

This is a relation like I'm using:
```ruby
class NS::ModuleName::MyClass
  belongs_to :other_class, class_name: NS::ModuleName::OtherClass.to_s, index: true, touch: true
end

class NS::ModuleName::OtherClass
  has_many :my_class, class_name: NS::ModuleName::MyClass.to_s, index: true, touch: true
end
```

While debugging the code, I've noticed the `get_inclusion_metadata` method was trying to `constantize` `"other_class"` instead of `"NS::ModuleName::OtherClass"`.

```
def get_inclusion_metadata(_klass, association)
  if _klass.is_a?(Class)
    _klass.reflect_on_association(association)
  else
    # here, _klass.to_s.classify was "OtherClass", while it should be "NS::ModuleName::OtherClass"
    _klass.to_s.classify.constantize.reflect_on_association(association)
  end
end
```

I'm not sure about performance impact though.

I need somebody who knows this codebase better to check it out.

Any thoughts?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant